Skip to content

Properly initalize counts array#59

Closed
nhorman wants to merge 1 commit into
mainfrom
56
Closed

Properly initalize counts array#59
nhorman wants to merge 1 commit into
mainfrom
56

Conversation

@nhorman

@nhorman nhorman commented Oct 21, 2025

Copy link
Copy Markdown
Contributor

The writeread test was producing wildly inconsistent results on windows.

Problem turned out to be the allocation of the counts array, which by some stroke of luck seems to always get a buffer full of zeros on most platforms...except for windows which more frequently gets whatever garbage is on the heap at the allocated location. This in turn leads to count values that start at some huge number and goes up from there, leading to 0 average per call run time results.

Allocate the array with zalloc to ensure 0 count values at the start of the test.

Fixes #56

The writeread test was producing wildly inconsistent results on windows.

Problem turned out to be the allocation of the counts array, which by
some stroke of luck seems to always get a buffer full of zeros on most
platforms...except for windows which more frequently gets whatever
garbage is on the heap at the allocated location.  This in turn leads to
count values that start at some huge number and goes up from there,
leading to 0 average per call run time results.

Allocate the array with zalloc to ensure 0 count values at the start of
the test.

Fixes #56

@npajkovsky npajkovsky left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦

@nhorman nhorman requested a review from quarckster October 21, 2025 20:36

@quarckster quarckster left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Sashan Sashan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice. looks good to me thanks.

Sashan pushed a commit that referenced this pull request Oct 22, 2025
The writeread test was producing wildly inconsistent results on windows.

Problem turned out to be the allocation of the counts array, which by
some stroke of luck seems to always get a buffer full of zeros on most
platforms...except for windows which more frequently gets whatever
garbage is on the heap at the allocated location.  This in turn leads to
count values that start at some huge number and goes up from there,
leading to 0 average per call run time results.

Allocate the array with zalloc to ensure 0 count values at the start of
the test.

Fixes #56

Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
(Merged from #59)
@Sashan

Sashan commented Oct 22, 2025

Copy link
Copy Markdown
Contributor

fixed via b7ec458

@Sashan Sashan closed this Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

writeread tests produce garbage on Windows

8 participants